-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Node.js to v16 and update frontend dependencies #519
Conversation
also changed shell to bash We use nvm to install Node.js as nodesource distributions somehow always install Node.js 18 instead (see nodesource/distributions#1583)
Also alias the default version If you want to see the full output during Docker build, you can use the "--progress=plain" option.
With this, I am getting (after I nuked all existing volumes and images and rebuilt everything) the following error from webpacker:
A similar message is also in the logs for experimental. |
Thanks, just stumbled on this too. We still need |
After updating This is probably related to rails/webpacker#2969 |
This is because we still use webpacker v4 currently. We will upgrade this depenedency when we switch from @rails/webpacker to webpack in the future.
This comment was marked as off-topic.
This comment was marked as off-topic.
This is so that Node.js is not installed again, which would result in v18, which we don't want at the moment.
This ensures that the app user can access node that was previously installed with nvm in /root/.nvm
Hey @christian-heusel, it'd be great if you could take a look at these changes and give a review as you have created the original Dockerfile ☺ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @christian-heusel, it'd be great if you could take a look at these changes and give a review as you have created the original Dockerfile
☺️
That is not correct, I have just heavily modified the setup!
4b5ca69 adds the dockerfile and was not done by me 😊
Anyways, I have left some comments about stuff that could be improved in my opinion, although I fear I am of no much help here 😄
Review is based on source code only, no actual testing has been done by me (yet).
Co-authored-by: Christian Heusel <christian@heusel.eu>
Oh, sorry ;) @christian-heusel Thanks a lot for your feedback!
Definitely not the case |
- Replaced Yarn installation with "corepack", a new binary shipped with Node.js starting with v16. This also means that Yarn is upgraded from v1 to v2 with a different CLI, e.g. we now use "yarn workspaces focus" instead of "yarn install" to install the dependencies for the workspace in the current working directory. - Update GPG key handling during PostgreSQL installation - Added some more comments to the Dockerfile - Moved copying of Node.js up in the Dockerfile More yarn-related changed in subsequent commits.
- Added a fix so that corepack really installs the version we want and not some other old version - with Yarn2+, building curerntly fails, so we switch back to Yarn1 and deal with an upgrade of Yarn in the future
Yarn1 does not generate this folder
"unnecessary" means dependencies that were added when trying out Yarn2+
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still some problems with this as the development
image will not build currently, for fixes see the comments 😊
Also great work on applying the previous suggestions! 🙌🏻 🥳
Co-authored-by: Christian Heusel <christian@heusel.eu>
(in prod Dockerfile)
@christian-heusel Thanks a lot for reviewing this PR 👍 |
* Use npm audit to update npm packages * Use Node.js version 16 in Docker development also changed shell to bash We use nvm to install Node.js as nodesource distributions somehow always install Node.js 18 instead (see nodesource/distributions#1583) * Use node after installation * Split installation of Node.js to multiple RUN commands Also alias the default version If you want to see the full output during Docker build, you can use the "--progress=plain" option. * Uniy sass packages and update yarn lock * Reflect changes in production and test Dockerfiles * Add comments to Dockerfile and tighten run commands * Add sass-loader back as dependency * Switch back to old version of sass-loader This is because we still use webpacker v4 currently. We will upgrade this depenedency when we switch from @rails/webpacker to webpack in the future. * Use "-no-install-recommends" for yarn install in prod This is so that Node.js is not installed again, which would result in v18, which we don't want at the moment. * Use consistent ${} syntax in Dockerfile & fix Node path * Copy Node.js over to /usr/local This ensures that the app user can access node that was previously installed with nvm in /root/.nvm * Apply quote arguments suggestion (in code review) Co-authored-by: Christian Heusel <christian@heusel.eu> * Quote arguments in other Dockerfiles as well * Replace deprecated apt-key and improve Dockerfiles - Replaced Yarn installation with "corepack", a new binary shipped with Node.js starting with v16. This also means that Yarn is upgraded from v1 to v2 with a different CLI, e.g. we now use "yarn workspaces focus" instead of "yarn install" to install the dependencies for the workspace in the current working directory. - Update GPG key handling during PostgreSQL installation - Added some more comments to the Dockerfile - Moved copying of Node.js up in the Dockerfile More yarn-related changed in subsequent commits. * Upgrade to Yarn 2 and add packages for non-error webpack build * Copy Yarn to usr folder alongside other Node.js tooling * Activate Yarn for app user * Only copy node over to usr folder * Go back to Yarn1 and explicitly set Yarn version - Added a fix so that corepack really installs the version we want and not some other old version - with Yarn2+, building curerntly fails, so we switch back to Yarn1 and deal with an upgrade of Yarn in the future * Remove ".yarn" from .gitignore Yarn1 does not generate this folder * Remove unnecessary dependencies from package.json "unnecessary" means dependencies that were added when trying out Yarn2+ * Add back missing "--production=false" to "yarn install" * Use "ruby:3.1.4-bullseye" to have PostgreSQL available * Add missing `apt update` in dev and test Dockerfiles Co-authored-by: Christian Heusel <christian@heusel.eu> * Group `update` and `install` in one RUN statement (in prod Dockerfile) * Remove superfluous newline * Make Node.js version logging one RUN statement * Explicitly set `--production=true` during `yarn install` --------- Co-authored-by: Christian Heusel <christian@heusel.eu>
Build in production suddenly failed with
and Node.js version was
Node.js v18.13.0
although we explicitly specified v14. This might be related to this issue. We now install Node.js v16 as the EOL of v14 was already reached in April this year. v16 is only maintained until the end of September 2023 (see here), so we should upgrade to v18 eventually, which is currently the LTS. We switched tonvm
to install Node.js as v18 was somehow installed although we specified v16, see the already mentioned issue.from here